-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FXIOS-10362 - Enable address autofill edit by default on US and CA #23256
Add FXIOS-10362 - Enable address autofill edit by default on US and CA #23256
Conversation
Client.app: Coverage: 31.54
Generated by 🚫 Danger Swift against fe49ec8 |
var isEditingFeatureEnabled: Bool { | ||
AddressLocaleFeatureValidator.isValidRegion() || | ||
featureFlags.isFeatureEnabled(.addressAutofillEdit, checking: .buildOnly) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@issammani won't this enable for all even if the reason is valid?
I am not sure if we want the logic to first check feature enabled then validate via region
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I am missing something featureFlags.isFeatureEnabled(.addressAutofillEdit, checking: .buildOnly)
should only be true if the user is enrolled in the experiment ? My reasoning to leave it there was because we want to rollout to germany and france at some point so we will need a nimbus flag again. But I can remove it from this check and keep the flag in the nimbus manifest for later use. What do you think ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, I understand why we need the || with your original change. Please change it back to the way it was before :)
18ba9b8
to
8f82957
Compare
8f82957
to
fe49ec8
Compare
@Mergifyio backport release/v133 |
✅ Backports have been created
|
#23256) chore: enable address edit by default on US and CA
#23256) chore: enable address edit by default on US and CA
…A (backport #23256) (#23337) Add FXIOS-10362 - Enable address autofill edit by default on US and CA (#23256) chore: enable address edit by default on US and CA (cherry picked from commit 6e1f58c) Co-authored-by: Issam Mani <[email protected]>
📜 Tickets
Jira ticket
Github issue
💡 Description
This PR:
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)